Skip to content

Conversation

@kasuga-fj
Copy link
Contributor

@kasuga-fj kasuga-fj commented Nov 28, 2025

In gcdMIVtest, there is logic that assumes the addition(s) of SCEVAddExpr don't overflow without any checks. Adding overflow checks would be fine, but this part appeart to be less useful. So this patch removes it.

Fix one of the tests added in #169926.

Copy link
Contributor Author

kasuga-fj commented Nov 28, 2025

@kasuga-fj kasuga-fj marked this pull request as ready for review November 29, 2025 01:44
@llvmbot llvmbot added the llvm:analysis Includes value tracking, cost tables and constant folding label Nov 29, 2025
@llvmbot
Copy link
Member

llvmbot commented Nov 29, 2025

@llvm/pr-subscribers-llvm-analysis

Author: Ryotaro Kasuga (kasuga-fj)

Changes

In gcdMIVtest, there is logic that assumes the addition(s) of SCEVAddExpr don't overflow without any checks. Adding overflow checks would be fine, but this part appeart to be less useful. So this patch removes it.

Fix one of the tests added in #169926.


Full diff: https://github.com/llvm/llvm-project/pull/169927.diff

2 Files Affected:

  • (modified) llvm/lib/Analysis/DependenceAnalysis.cpp (-17)
  • (modified) llvm/test/Analysis/DependenceAnalysis/gcd-miv-overflow.ll (+4-5)
diff --git a/llvm/lib/Analysis/DependenceAnalysis.cpp b/llvm/lib/Analysis/DependenceAnalysis.cpp
index 77b09fb15316e..bb54c0986c20f 100644
--- a/llvm/lib/Analysis/DependenceAnalysis.cpp
+++ b/llvm/lib/Analysis/DependenceAnalysis.cpp
@@ -2590,23 +2590,6 @@ bool DependenceInfo::gcdMIVtest(const SCEV *Src, const SCEV *Dst,
   const SCEV *Delta = SE->getMinusSCEV(DstConst, SrcConst);
   LLVM_DEBUG(dbgs() << "    Delta = " << *Delta << "\n");
   const SCEVConstant *Constant = dyn_cast<SCEVConstant>(Delta);
-  if (const SCEVAddExpr *Sum = dyn_cast<SCEVAddExpr>(Delta)) {
-    // If Delta is a sum of products, we may be able to make further progress.
-    for (const SCEV *Operand : Sum->operands()) {
-      if (isa<SCEVConstant>(Operand)) {
-        assert(!Constant && "Surprised to find multiple constants");
-        Constant = cast<SCEVConstant>(Operand);
-      } else if (const SCEVMulExpr *Product = dyn_cast<SCEVMulExpr>(Operand)) {
-        // Search for constant operand to participate in GCD;
-        // If none found; return false.
-        std::optional<APInt> ConstOp = getConstanCoefficient(Product);
-        if (!ConstOp)
-          return false;
-        ExtraGCD = APIntOps::GreatestCommonDivisor(ExtraGCD, ConstOp->abs());
-      } else
-        return false;
-    }
-  }
   if (!Constant)
     return false;
   APInt ConstDelta = cast<SCEVConstant>(Constant)->getAPInt();
diff --git a/llvm/test/Analysis/DependenceAnalysis/gcd-miv-overflow.ll b/llvm/test/Analysis/DependenceAnalysis/gcd-miv-overflow.ll
index 618d14c75faad..f6b23f18df738 100644
--- a/llvm/test/Analysis/DependenceAnalysis/gcd-miv-overflow.ll
+++ b/llvm/test/Analysis/DependenceAnalysis/gcd-miv-overflow.ll
@@ -127,9 +127,8 @@ exit:
 ;   A[-3*i - 3*m - INT64_MAX] = 1;
 ; }
 ;
-; FIXME: DependenceAnalysis currently detects no dependency between the two
-; stores, but it may exist. For example, consider `m = 1`. Then,
-; `-3*m - INT64_MAX` is `INT64_MAX - 1`. So `-3*i - 3*m - INT64_MAX` is
+; Dependency may exist between the two stores. For example, consider `m = 1`.
+; Then, `-3*m - INT64_MAX` is `INT64_MAX - 1`. So `-3*i - 3*m - INT64_MAX` is
 ; effectively `-3*i + (INT64_MAX - 1)`. Thus, accesses will be as follows:
 ;
 ;  memory access             | i == 1           | i == max_i - 1
@@ -143,7 +142,7 @@ define void @gcdmiv_const_ovfl(ptr %A, i64 %m) {
 ; CHECK-ALL-NEXT:  Src: store i8 0, ptr %gep.0, align 1 --> Dst: store i8 0, ptr %gep.0, align 1
 ; CHECK-ALL-NEXT:    da analyze - none!
 ; CHECK-ALL-NEXT:  Src: store i8 0, ptr %gep.0, align 1 --> Dst: store i8 1, ptr %gep.1, align 1
-; CHECK-ALL-NEXT:    da analyze - none!
+; CHECK-ALL-NEXT:    da analyze - output [*|<]!
 ; CHECK-ALL-NEXT:  Src: store i8 1, ptr %gep.1, align 1 --> Dst: store i8 1, ptr %gep.1, align 1
 ; CHECK-ALL-NEXT:    da analyze - none!
 ;
@@ -151,7 +150,7 @@ define void @gcdmiv_const_ovfl(ptr %A, i64 %m) {
 ; CHECK-GCD-MIV-NEXT:  Src: store i8 0, ptr %gep.0, align 1 --> Dst: store i8 0, ptr %gep.0, align 1
 ; CHECK-GCD-MIV-NEXT:    da analyze - consistent output [*]!
 ; CHECK-GCD-MIV-NEXT:  Src: store i8 0, ptr %gep.0, align 1 --> Dst: store i8 1, ptr %gep.1, align 1
-; CHECK-GCD-MIV-NEXT:    da analyze - none!
+; CHECK-GCD-MIV-NEXT:    da analyze - consistent output [*|<]!
 ; CHECK-GCD-MIV-NEXT:  Src: store i8 1, ptr %gep.1, align 1 --> Dst: store i8 1, ptr %gep.1, align 1
 ; CHECK-GCD-MIV-NEXT:    da analyze - consistent output [*]!
 ;

Copy link
Member

@Meinersbur Meinersbur left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

kasuga-fj added a commit that referenced this pull request Dec 1, 2025
…169926)

Add two test cases where dependencies are missed due to overflows. These
will be fixed by #169927 and #169928, respectively.
Base automatically changed from users/kasuga-fj/da-gcdmiv-delta-overflow-test to main December 1, 2025 13:36
@kasuga-fj kasuga-fj force-pushed the users/kasuga-fj/da-gcdmiv-remove-special-handling branch from 10136ad to 575f811 Compare December 1, 2025 13:39
@kasuga-fj kasuga-fj force-pushed the users/kasuga-fj/da-gcdmiv-remove-special-handling branch from 575f811 to 6b3d008 Compare December 1, 2025 13:45
aahrun pushed a commit to aahrun/llvm-project that referenced this pull request Dec 1, 2025
…lvm#169926)

Add two test cases where dependencies are missed due to overflows. These
will be fixed by llvm#169927 and llvm#169928, respectively.
@kasuga-fj kasuga-fj enabled auto-merge (squash) December 1, 2025 14:08
@kasuga-fj kasuga-fj merged commit fa6d611 into main Dec 1, 2025
9 of 10 checks passed
@kasuga-fj kasuga-fj deleted the users/kasuga-fj/da-gcdmiv-remove-special-handling branch December 1, 2025 14:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

llvm:analysis Includes value tracking, cost tables and constant folding

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants